Restore handling of 204 "soft" redirects on data requests #13364
Conversation
🦋 Changeset detectedLatest commit: 748bf13 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
integration/single-fetch-test.ts
Outdated
| res.statusMessage = response.statusText; | ||
| res.status(response.status); | ||
| for (let [key, value] of response.headers.entries()) { | ||
| res.append(key, value); | ||
| } |
There was a problem hiding this comment.
We don't export our internal logic for converting a fetch Response to an express res so for now it's assumed that the application has to do that adapter logic on their own. I think this is ok since status/headers are all that matter? There isn't a body on these responses
| } else { | ||
| return { status: SINGLE_FETCH_REDIRECT_STATUS, data }; | ||
| } | ||
| } |
There was a problem hiding this comment.
If we get back a 204 redirect we convert it to a single fetch redirect here for downstream processing
| } | ||
| if (result.replace) { | ||
| headers["X-Remix-Replace"] = "yes"; | ||
| headers["X-Remix-Replace"] = "true"; |
There was a problem hiding this comment.
Align these with values used elsewhere - value doesn't really matter since we use .has(header)
brophdawg11
changed the title
|
I'm a fan. Will this be documented unstable behavior? What's the motivation for not stabilizing the |
Not likely - the goal is parity with pre-single fetch to ease upgrading. It was undocumented/unstable before, and will be undocumented/unstable here as well. That said - these things are highly unlikely to change and it should be relatively easy to add an integration test that would fail if we did change a header name or something.
Now that you can do these types of "before the handlers run" redirects from a RR middleware, I don't know that we want to further encourage doing this outside of the app |
| // client side router. We use a 202 to avoid any automatic caching we might | ||
| // get from a 200 since a "temporary" redirect should not be cached. This lets | ||
| // the user control cache behavior via Cache-Control | ||
| export const SINGLE_FETCH_REDIRECT_STATUS = 202; |
There was a problem hiding this comment.
Pulled into here from the server-runtime file to avoid circular deps
tl;dr;
.datarequests now understand the 204 redirects we used in Remix v2 before Single FetchredirectBackground:
In order to trigger a redirect navigation from a client side
fetch, we have to use something other than a 3xx response because a 3xx will redirect thefetchcall. We need to return some form of "data" that we can look at and decide to perform a soft application redirect.Prior to single fetch, we used to return 204 responses with
X-Remix-Redirectheaders. These worked well and used web standards so while the 204 code and custom header were "implementation details", folks could use them if/when needed to perform redirects on_datarequests from outside of Remix - i.e., in an express middleware.With single fetch, we switched to encoding the redirect into the body of the response so we could encode them in prerendered
.datafiles as well. This means that a redirect on a.datarequest relies on the implementation detila of turbo-stream's encoding format.This made redirects on
.datarequests from outside of React Router much tricker, since they now need to encode aturbo-streamresponse body and use the custom symbol we use internally (#12693).Options: We have 2 options if we want to re-enable this functionality:
turbo-streamThis PR adds back support for those 204 responses so folks can redirect the same way they did in Remix v2.
Closes #12693